Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

Hello @mwhooker, thank you for opening this one. I was taking a look at this some time ago and was wondering what would be a users expectation. If there is a dial timeout, should it include the total amount of time for a connection to be established (including retries) or should it be just for a single dial. Based on your PR you would expect this to be per single connection, but does it make sense to have one that is for the whole period?

I think this change makes sense since this simplifies library's user expectation. dialTimeout config always means per dial/connection call. So math of total timeout based on retryCount is easier to do on the user side before choosing values. Currently, prior to this change, I feel it makes it complicated for users to choose because I can imagine when tweaking values for more retries (during prod incident resolution etc) it may be easily forgotten that all retries have to cumulatively meet single dialTimeout config value. To me that is counter-intuitive. Also, this may bring Redis go lib to parity with many other client libs like redis-py and provide consistency for users.

@maulin-vasavada we do have a slightly different way of opening connection and retrying compared to redis-py, but i do agree that this can be confusing. will have to run this with our e2e tests around end of week to verify it works as expected in different scenarious.

Sorry for the delay, the release we just did took longer than expected. Anyway, this week I will have some cycles to look at this and get back to you.